Skip to content

Conversation

@hannesrudolph
Copy link
Collaborator

Summary

This PR fixes an issue where the browserToolEnabled setting was not correctly affecting the system prompt generation, causing the browser functionality to either always be included or always be excluded regardless of user preference.

Issue Details

When users toggled the "Enable Browser Tools" setting (browserToolEnabled) in the settings panel, this change was not properly reflected in the system prompt. The root cause was in the generateSystemPrompt function, which was only considering the model's capability (supportsComputerUse) but not checking the user's preference from state (browserToolEnabled).

Technical Analysis

The issue occurred because:

  1. The generateSystemPrompt function wasn't retrieving the browserToolEnabled setting from state
  2. It was passing only the model capability check (apiConfiguration.openRouterModelInfo?.supportsComputerUse) to the SYSTEM_PROMPT function
  3. This meant the user setting had no effect on the system prompt generation

This is a pattern we need to watch for in the codebase - any place where we need to check both:

  • Model capability (from getCurrentCline() or apiConfiguration)
  • User preference (from state)

Fix Implementation

The fix:

  1. Retrieves the browserToolEnabled setting from state in generateSystemPrompt
  2. Creates a combined check: canUseBrowserTool = (apiConfiguration.openRouterModelInfo?.supportsComputerUse ?? false) && (browserToolEnabled ?? true)
  3. Passes this combined value to SYSTEM_PROMPT

This ensures that browser tools are only enabled when BOTH the model supports computer use AND the user has enabled browser tools.

Additional Improvements

  • Added proper error handling and fallbacks in the telemetry properties collection to handle undefined Cline instances
  • Improved the pattern for accessing model capabilities through apiConfiguration when no Cline instance exists

Testing

  • Added tests that directly verify the canUseBrowserTool computation in various scenarios
  • Added integration tests to verify the entire flow from setting changes to system prompt generation

These test improvements should catch similar issues in the future.

Context

Implementation

Screenshots

before after

How to Test

Get in Touch

## Summary
This PR fixes an issue where the browserToolEnabled setting was not correctly affecting the system prompt generation, causing the browser functionality to either always be included or always be excluded regardless of user preference.

## Issue Details
When users toggled the "Enable Browser Tools" setting (browserToolEnabled) in the settings panel, this change was not properly reflected in the system prompt. The root cause was in the `generateSystemPrompt` function, which was only considering the model's capability (`supportsComputerUse`) but not checking the user's preference from state (`browserToolEnabled`).

## Technical Analysis
The issue occurred because:

1. The `generateSystemPrompt` function wasn't retrieving the `browserToolEnabled` setting from state
2. It was passing only the model capability check (`apiConfiguration.openRouterModelInfo?.supportsComputerUse`) to the `SYSTEM_PROMPT` function
3. This meant the user setting had no effect on the system prompt generation

This is a pattern we need to watch for in the codebase - any place where we need to check both:
- Model capability (from `getCurrentCline()` or `apiConfiguration`)
- User preference (from state)

## Fix Implementation
The fix:
1. Retrieves the `browserToolEnabled` setting from state in `generateSystemPrompt`
2. Creates a combined check: `canUseBrowserTool = (apiConfiguration.openRouterModelInfo?.supportsComputerUse ?? false) && (browserToolEnabled ?? true)`
3. Passes this combined value to `SYSTEM_PROMPT`

This ensures that browser tools are only enabled when BOTH the model supports computer use AND the user has enabled browser tools.

## Additional Improvements
- Added proper error handling and fallbacks in the telemetry properties collection to handle undefined Cline instances
- Improved the pattern for accessing model capabilities through apiConfiguration when no Cline instance exists

## Testing
- Added tests that directly verify the `canUseBrowserTool` computation in various scenarios
- Added integration tests to verify the entire flow from setting changes to system prompt generation

These test improvements should catch similar issues in the future.
@changeset-bot
Copy link

changeset-bot bot commented Mar 17, 2025

⚠️ No Changeset found

Latest commit: 4595017

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR


// Check if browser tool is enabled by both model capability and user preference
const canUseBrowserTool =
(apiConfiguration.openRouterModelInfo?.supportsComputerUse ?? false) && (browserToolEnabled ?? true)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hannesrudolph this is just what we had before. didn't work as well. It checks only for Open Router, no? What if the user is not using open router?

@cannuri
Copy link
Contributor

cannuri commented Mar 21, 2025

I submitted a PR for this: #1840

@hannesrudolph hannesrudolph moved this from New to PR [Pre Approval Review] in Roo Code Roadmap Mar 21, 2025
@github-project-automation github-project-automation bot moved this from PR [Pre Approval Review] to Done in Roo Code Roadmap Mar 26, 2025
@hannesrudolph hannesrudolph deleted the browsertools-fix branch May 12, 2025 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants